[client] Add local Prometheus metrics endpoint - #6689
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughAdds a local Prometheus ChangesLocal metrics feature
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: ⚪ Minimal · up to The PR adds an opt-in local metrics endpoint and related configuration without any identified merge-blocking risk; it is merge-ready after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant CLI
participant Server
participant LocalMetrics
participant ClientEngine
participant Prometheus
CLI->>Server: Configure local metrics
Server->>LocalMetrics: Reconcile enabled state and address
LocalMetrics->>ClientEngine: Request Prometheus gatherer
Prometheus->>LocalMetrics: GET /metrics
LocalMetrics->>ClientEngine: Gather engine metrics
LocalMetrics-->>Prometheus: Return local and engine metrics
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description explains the feature, implementation scope, tests, and documentation. However, the required issue ticket or approved discussion link is missing, and the checklist contains a contradictory claim that the change does not introduce a new feature or modify CLI flags.
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
client/internal/localmetrics/localmetrics.go (1)
60-63: 🔒 Security & Privacy | 🔵 TrivialConsider warning when binding to a non-loopback address.
The endpoint exposes peer FQDNs, latency, and connectivity state without authentication. Default is loopback-only, but
addris fully user-controlled; a log warning (or explicit opt-in) when the host isn't loopback would help users avoid accidental exposure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/internal/localmetrics/localmetrics.go` around lines 60 - 63, The local metrics endpoint in Manager.Reconcile can bind to any user-supplied addr, which may accidentally expose sensitive peer and connectivity data. Update Reconcile to detect when addr is not loopback after defaulting to DefaultListenAddress, and emit a clear warning (or require explicit opt-in) before starting the listener. Keep the check near the address selection logic so the behavior is obvious and easy to maintain.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/internal/localmetrics/localmetrics.go`:
- Line 94: The local metrics server created in `Start` uses `http.Server`
without any timeout protections, leaving it vulnerable to slow client
connections. Update the `srv := &http.Server{...}` construction to set
appropriate `ReadTimeout`, `ReadHeaderTimeout`, `WriteTimeout`, and
`IdleTimeout` values, keeping the existing `Addr` and `Handler` behavior intact.
Use the `Start` function and the `http.Server` setup as the place to apply these
limits.
---
Nitpick comments:
In `@client/internal/localmetrics/localmetrics.go`:
- Around line 60-63: The local metrics endpoint in Manager.Reconcile can bind to
any user-supplied addr, which may accidentally expose sensitive peer and
connectivity data. Update Reconcile to detect when addr is not loopback after
defaulting to DefaultListenAddress, and emit a clear warning (or require
explicit opt-in) before starting the listener. Keep the check near the address
selection logic so the behavior is obvious and easy to maintain.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7f956574-47e5-405e-a5e6-4581c2baf2ed
⛔ Files ignored due to path filters (1)
client/proto/daemon.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (15)
client/cmd/root.goclient/cmd/up.goclient/internal/localmetrics/localmetrics.goclient/internal/localmetrics/localmetrics_test.goclient/internal/metrics/influxdb.goclient/internal/metrics/metrics.goclient/internal/metrics/metrics_default.goclient/internal/metrics/prometheus.goclient/internal/peer/status.goclient/internal/profilemanager/config.goclient/proto/daemon.protoclient/server/server.goclient/server/setconfig_test.gogo.modinfrastructure_files/observability/grafana/dashboards/client.json
Release artifactsBuilt for PR head
GHCR images (amd64)
This comment is updated by the Release workflow. Artifact links expire according to the workflow retention policy. |
cd0ecae to
6d9cb90
Compare
35e157d to
e93fd6c
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/server/mdm.go`:
- Around line 304-305: Preserve nil-versus-explicit-empty presence when checking
LocalMetricsAddress by replacing the conflictString calls using
msg.GetLocalMetricsAddress() with a pointer-aware comparison. Apply this change
at client/server/mdm.go lines 304-305 and 431-432, while retaining the existing
managed-field conflict behavior for both absent and supplied values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: e464b486-7137-49d6-a214-0c83007fc044
⛔ Files ignored due to path filters (1)
client/proto/daemon.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (20)
client/cmd/root.goclient/cmd/up.goclient/internal/debug/debug.goclient/internal/localmetrics/localmetrics.goclient/internal/localmetrics/localmetrics_test.goclient/internal/metrics/influxdb.goclient/internal/metrics/metrics.goclient/internal/metrics/metrics_default.goclient/internal/metrics/prometheus.goclient/internal/peer/status.goclient/internal/profilemanager/config.goclient/internal/profilemanager/config_mdm_test.goclient/mdm/policy.goclient/proto/daemon.protoclient/server/mdm.goclient/server/server.goclient/server/setconfig_mdm_test.goclient/server/setconfig_test.gogo.modinfrastructure_files/observability/grafana/dashboards/client.json
🚧 Files skipped from review as they are similar to previous changes (18)
- client/internal/localmetrics/localmetrics_test.go
- infrastructure_files/observability/grafana/dashboards/client.json
- client/server/setconfig_mdm_test.go
- client/server/setconfig_test.go
- client/internal/profilemanager/config_mdm_test.go
- client/internal/metrics/metrics.go
- client/cmd/up.go
- client/internal/localmetrics/localmetrics.go
- client/internal/metrics/metrics_default.go
- client/internal/metrics/influxdb.go
- client/internal/profilemanager/config.go
- client/internal/debug/debug.go
- client/cmd/root.go
- client/internal/metrics/prometheus.go
- client/proto/daemon.proto
- client/internal/peer/status.go
- go.mod
- client/server/server.go
# Conflicts: # client/internal/debug/debug.go
|
❌ The last analysis has failed. |
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
1 similar comment
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
theodorsm
left a comment
There was a problem hiding this comment.
Foreground mode accepts the local metrics flags, but does not serve the endpoint. Do we want to support this feature for foreground mode?
| ch <- prometheus.MustNewConstMetric(c.signalConnected, prometheus.GaugeValue, boolToFloat(c.status.GetSignalState().Connected)) | ||
|
|
||
| peers := c.status.GetPeerStates() | ||
| ch <- prometheus.MustNewConstMetric(c.peersTotal, prometheus.GaugeValue, float64(len(peers))) |
There was a problem hiding this comment.
| d.mux.RLock() | ||
| defer d.mux.RUnlock() | ||
|
|
||
| states := make([]State, 0, len(d.peers)) |
There was a problem hiding this comment.
Should we include offline peers too, like GetFullStatus ? Ref: https://github.com/netbirdio/netbird/pull/6689/changes#r3863137373
| KeyEnableLocalMetrics = "enableLocalMetrics" | ||
| KeyLocalMetricsAddress = "localMetricsAddress" |
There was a problem hiding this comment.
KeyEnableLocalMetrics and KeyLocalMetricsAddress are never added to allKeys (client/mdm/canonical_loaders.go), so the Windows registry and macOS plist loaders discard them as unknown.
| return nil, fmt.Errorf("failed to get default profile config: %w", err) | ||
| } | ||
|
|
||
| s.config = config |
There was a problem hiding this comment.
Missing localMetrics.Reconcile. The profile we switch to might have different metrics settings.
| return nil, fmt.Errorf("failed to get active profile config: %w", err) | ||
| } | ||
| s.mutex.Lock() | ||
| s.config = config |
There was a problem hiding this comment.
Missing localMetrics.Reconcile. The profile might have changed.
| log.Infof("serving local metrics on http://%s/metrics", addr) | ||
| go func() { | ||
| if err := srv.ListenAndServe(); err != nil && !errors.Is(err, http.ErrServerClosed) { | ||
| log.Errorf("failed to serve local metrics on %s: %v", addr, err) |
There was a problem hiding this comment.
We should reset the m.srv and m.addr fields like we do in stop().
| if addr == "" { | ||
| addr = DefaultListenAddress | ||
| } | ||
| warnIfNotLoopback(addr) |
There was a problem hiding this comment.
We should gate it behind a privilege check like we do for ssh rather than just logging.
|
This PR contains the following updates: | Package | Update | Change | |---|---|---| | [netbirdio/netbird](https://github.com/netbirdio/netbird) | minor | `0.77.1` → `0.78.1` | [Release notes](https://github.com/netbirdio/netbird/releases) --- ### Release Notes <details> <summary>netbirdio/netbird (netbirdio/netbird)</summary> ### [`v0.78.1`](https://github.com/netbirdio/netbird/releases/tag/v0.78.1) [Compare Source](netbirdio/netbird@v0.78.0...v0.78.1) #### What's Changed - \[management] Serve networks with peer-based routers from the SQLite network map by [@​mlsmaycon](https://github.com/mlsmaycon) in [#​7424](netbirdio/netbird#7424) **Full Changelog**: <netbirdio/netbird@v0.78.0...v0.78.1> ### [`v0.78.0`](https://github.com/netbirdio/netbird/releases/tag/v0.78.0) [Compare Source](netbirdio/netbird@v0.77.1...v0.78.0) #### Release Notes for v0.78.0 #### Highlights - **Rosenpass now works through the embedded reverse proxy** ([#​6763](netbirdio/netbird#6763)). Until now the proxy ran no Rosenpass at all, so peers with Rosenpass enabled failed to establish through it on a PSK mismatch — an incompatibility that was never documented. The proxy now runs Rosenpass in **permissive** mode, so it connects both to Rosenpass-enabled peers and, exactly as before, to peers without it. `NB_PROXY_ROSENPASS=false` turns it off. - **Lazy connections reworked**: per-peer lazy state, proxy peers lazy by default, and the lazy exclusion list removed ([#​6762](netbirdio/netbird#6762), [#​6763](netbirdio/netbird#6763)). - **Agent Network / LLM gateway**: agentgateway integration, access roles and self-service endpoints, Bedrock model discovery served from the control plane, guardrail allowlists for declared model ids, and the endpoint conformed to the LLM gateway protocol ([#​7274](netbirdio/netbird#7274), [#​7221](netbirdio/netbird#7221), [#​7250](netbirdio/netbird#7250), [#​7239](netbirdio/netbird#7239), [#​7154](netbirdio/netbird#7154), [#​7389](netbirdio/netbird#7389), [#​7246](netbirdio/netbird#7246)). - **DNS on Windows**: a catch-all NRPT rule when NetBird is the primary resolver ([#​7071](netbirdio/netbird#7071)), closing the leak/poisoning window towards the system resolvers. Use `netbird service reconfigure --service-env NB_USE_LEGACY_DNS_RESOLUTION=true` to restore the old behavior. - **Local Prometheus metrics endpoint** on the client ([#​6689](netbirdio/netbird#6689)). - **Go 1.26** and `go-quic` v0.62.0 across client, relay and management ([#​7359](netbirdio/netbird#7359)). - **Unified ACL filtering** for peers and routes, with multi-source rules ([#​6322](netbirdio/netbird#6322)). An internal refactor: no change is expected for standard deployments. - **Ukrainian localization** for the desktop client ([#​7035](netbirdio/netbird#7035)). #### Behaviour changes - Remote jobs (remote debug bundle and friends) are now **behind an admin opt-in**, with MDM support ([#​7153](netbirdio/netbird#7153)). Anyone using them without the opt-in has to enable it. - Remote debug bundle jobs accept an anonymization level and an upload URL ([#​7147](netbirdio/netbird#7147)). - The client stays connected during the `login` command ([#​7384](netbirdio/netbird#7384)). - Logging out of the active profile is allowed even when profiles are disabled ([#​7360](netbirdio/netbird#7360)). - Profiles resolve for the invoking `sudo` user rather than for `root` ([#​7238](netbirdio/netbird#7238)). - NetBird traffic stays out of third-party fwmark rules ([#​7314](netbirdio/netbird#7314)). - GUI windows are created on demand and destroyed on close ([#​7096](netbirdio/netbird#7096)). - Android split tunnelling: the mode is typed rather than stored as a string, and settings are kept per profile ([#​7387](netbirdio/netbird#7387), [#​7349](netbirdio/netbird#7349)). #### Security / hardening - The cached SSH JWT is bound to the local caller that obtained it ([#​7378](netbirdio/netbird#7378)). - The WireGuard key is no longer logged on a parse failure ([#​7379](netbirdio/netbird#7379)). - The client asks the OS for privileges when a guarded SSH setting is changed ([#​7066](netbirdio/netbird#7066)). - The proxy validates header auth ([#​7263](netbirdio/netbird#7263)). - Management checks a provider's URL and credential before saving them ([#​7301](netbirdio/netbird#7301)). - Clarified that `X-Peer-ID` on metrics ingest is not a credential ([#​7363](netbirdio/netbird#7363)). - The old `math/rand` library is gone from management ([#​6836](netbirdio/netbird#6836)). #### Client — fixes - Fixed the ICEBind races that wedge interface creation ([#​7377](netbirdio/netbird#7377)). - `agentConnecting` is dropped whenever the ICE session state clears ([#​7327](netbirdio/netbird#7327)). - A peer offer or answer arriving before the handshaker starts listening is held rather than lost ([#​7255](netbirdio/netbird#7255)). - Connections are swept on network loss through a shared netevents manager ([#​7254](netbirdio/netbird#7254)). - Route selection survives an invalid request and is applied on a partial one ([#​7292](netbirdio/netbird#7292)). - The session-expiration dialog closes only on renewal ([#​7337](netbirdio/netbird#7337)). - A still-locked updater binary is tolerated when cleaning up after an update ([#​7286](netbirdio/netbird#7286)). - Fixed context cancellation during restart on iOS ([#​7329](netbirdio/netbird#7329)). - iOS SSO logins reuse the profile's account ([#​7193](netbirdio/netbird#7193)). - The iOS profile manager was migrated from Swift to Go ([#​6528](netbirdio/netbird#6528)). - The PCP implementation moved to the go-nat fork ([#​7282](netbirdio/netbird#7282)). - Reverted multi-buffer support declared for the loopback XDP program ([#​7303](netbirdio/netbird#7303)). - The Android TUN is renewed only when the routes it carries actually change ([#​7396](netbirdio/netbird#7396)). - Overlay listeners are rebuilt when the TUN is renewed ([#​7397](netbirdio/netbird#7397)). - The remote jobs opt-in is exposed in the Android and iOS SDK preferences ([#​7406](netbirdio/netbird#7406)). #### Management — fixes - Fixed geolocation panics ([#​7382](netbirdio/netbird#7382)). - Fixed private services calculation on the new db path ([#​7383](netbirdio/netbird#7383)). - Fixed posture check evaluation for direct peers in policy definitions ([#​7348](netbirdio/netbird#7348)) and the affected peers calculation on a posture check flip ([#​7347](netbirdio/netbird#7347)). - Handled the nil pointer in `sendInitialSync()` when the peer has been deleted ([#​7315](netbirdio/netbird#7315)). - Network map from the nmap data type ([#​6919](netbirdio/netbird#6919)). #### Self-hosted / infrastructure - Better domain, Docker Compose and license validation in the self-hosted scripts ([#​7339](netbirdio/netbird#7339)). - The dashboard wasm client bump is triggered by release tags ([#​7277](netbirdio/netbird#7277)). - Protobuf breaking-change checks in CI ([#​7305](netbirdio/netbird#7305)). - Pinned the toolchain `gomobile init` needs for gobind ([#​7291](netbirdio/netbird#7291)). - Removed the mobile build validation workflow ([#​7302](netbirdio/netbird#7302)). #### Upgrade notes - **Remote debug bundles now require an explicit opt-in** ([#​7153](netbirdio/netbird#7153)). Bundles requested by the management server no longer run on a peer unless remote jobs are enabled there, with `--allow-remote-jobs` on the client or the `allowRemoteJobs` managed setting. Deployments relying on management-triggered debug bundles must opt in before they work again. The upload destination can now be pinned by the operator, with MDM taking precedence over the management-supplied value ([#​7147](netbirdio/netbird#7147)). - The embedded proxy now runs Rosenpass in permissive mode ([#​6763](netbirdio/netbird#6763)). Peers with Rosenpass enabled can now use the reverse proxy, which previously failed on a PSK mismatch; peers without Rosenpass keep connecting exactly as before. `NB_PROXY_ROSENPASS=false` disables it. - Proxy peers now default to lazy connections ([#​6762](netbirdio/netbird#6762)). Nothing else requires action. **Full Changelog**: <netbirdio/netbird@v0.77.1...v0.78.0> </details> --- ### Configuration 📅 **Schedule**: (UTC) - Branch creation - At any time (no schedule defined) - Automerge - At any time (no schedule defined) 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate CLI](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0NC4zMC40IiwidXBkYXRlZEluVmVyIjoiNDQuNjEuNiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsibWlub3IiLCJyZW5vdmF0ZSJdfQ==--> Reviewed-on: https://gitea.vcasaserver.com/omar/swarm/pulls/808 Co-authored-by: Renovate Bot <renovate-bot@vcasaserver.com>





Adds an opt-in Prometheus
/metricsendpoint to the client daemon so deployments can scrape connection health locally (requested for a RedHat evaluation: latency, connected peers, P2P vs relay).netbird up --enable-local-metricsand--local-metrics-address(default127.0.0.1:9191); the setting persists in the profile config and travels through SetConfig/Login like other config flags--disable-update-settingsand MDM policy (enableLocalMetrics,localMetricsAddresskeys)All labels are bounded enums except the per-peer latency series, which carries the peer FQDN only.
Issue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
netbirdio/docs#952
Summary by CodeRabbit
/metricsendpoint for client connectivity, peer states, latency, and connection performance.